packaging: added build scripts for Ubuntu and Fedora - #226
Conversation
|
Thanks, this is a great start! I'm reluctant to add a new external dependency on |
Thanks for the feedback! I completely understand. I'll switch over to using native tools and will have an update ready soon. |
mgree
left a comment
mgree
left a comment
There was a problem hiding this comment.
Took a quick look. Looking much better, but there are a few things I don't understand/could be cleaned up.
mgree
left a comment
mgree
left a comment
There was a problem hiding this comment.
Just a few tiny tweaks---thanks!
| - name: Build packaging image | ||
| run: | | ||
| docker build -t try-packager packaging/ | ||
|
|
||
| - name: Build .deb and .rpm | ||
| run: | | ||
| docker run --rm -v "$PWD:/work" -w /work try-packager packaging/build_packages.sh |
There was a problem hiding this comment.
(Sorry, I meant to leave this comment last time but somehow didn't write it down.)
Why bother using docker for this? We already have a clean VM from the CI runner, so we should be able to just invoke the builds script here in CI.... right?
There was a problem hiding this comment.
should be resolved now! Yes there was no need to use docker for this.
| - name: Check tag against declared versions | ||
| run: | | ||
| TAG_VERSION="${GITHUB_REF_NAME#v}" | ||
| SCRIPT_VERSION="$(grep 'TRY_VERSION=' try | cut -d'"' -f 2)" | ||
| MANPAGE_VERSION="$(grep 'TRY(1)' docs/try.1.md | cut -d' ' -f 4)" | ||
| INCLUDE_VERSION="$(grep '#define TRY_VERSION' utils/version.h | cut -d'"' -f 2)" | ||
| CONFIGAC_VERSION="$(grep AC_INIT configure.ac | cut -d'[' -f3 | cut -d']' -f1)" | ||
|
|
||
| echo " TAG_VERSION = '$TAG_VERSION'" | ||
| echo " SCRIPT_VERSION = '$SCRIPT_VERSION'" | ||
| echo " MANPAGE_VERSION = '$MANPAGE_VERSION'" | ||
| echo " INCLUDE_VERSION = '$INCLUDE_VERSION'" | ||
| echo "CONFIGAC_VERSION = '$CONFIGAC_VERSION'" | ||
|
|
||
| for v in "$SCRIPT_VERSION" "$MANPAGE_VERSION" "$INCLUDE_VERSION" "$CONFIGAC_VERSION" | ||
| do | ||
| if [ "$v" != "$TAG_VERSION" ] | ||
| then | ||
| echo "::error::tag $GITHUB_REF_NAME does not match all declared versions" | ||
| exit 1 | ||
| fi | ||
| done |
There was a problem hiding this comment.
(Duplicating a response from the prior review, just so everything is in one place.)
We already run scripts/check_version.sh as part of CI---so it's sufficient in tag-version-check to compare against the installed ones. No need to more or less duplicate the logic of the version-checking script in the YAML file.
I think the cleanest thing to do is to have check_version.sh take an optional argument indicating the expected version, and we can run the version check again at that extracted tag.
| dist-packages/*.deb | ||
| dist-packages/*.rpm |
There was a problem hiding this comment.
Elsewhere we're careful to name try-*.tgz or whatever, so that we don't glob up other things. Probably good to do that here (and later, when we install things during the smoke tests).
There was a problem hiding this comment.
This is going to only build x86_64 debs, since that's what ubuntu-latest is going to run on. I don't know that we need to bother building for aarch64/arm64, but wanted to note this.
2 participants
Adds .deb (Ubuntu) and .rpm (Fedora) build scripts